Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: fix test local edge case #52702

Closed
wants to merge 2 commits into from
Closed

src: fix test local edge case #52702

wants to merge 2 commits into from

Conversation

ShogunPanda
Copy link
Contributor

This PR fixes a edge case for a test which will fail if the .env file exists in the developer local worktree.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 26, 2024
benjamingr
benjamingr reviewed Apr 27, 2024
View reviewed changes
test/parallel/test-process-load-env-file.js Outdated
assert.throws(() => {
process.loadEnvFile();
}, { code: 'ENOENT', syscall: 'open', path: '.env' });

process.chdir(originalCwd);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this in beforeEach/aafterEach or at least in a a try/finally so if the test fails it doesn't cause unrelated failures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll go for a try-finally tomorrow morning!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjamingr Fixed. What about now?

@ShogunPanda ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 3, 2024

Failed to start CI
   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/8938415767

@github-actions github-actions bot added the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label May 3, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58893/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58914/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58938/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58942/

@marco-ippolito marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels May 5, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 5, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in b77f32b...de7b378

nodejs-github-bot pushed a commit that referenced this pull request May 5, 2024
@ShogunPanda @nodejs-github-bot
src: fix test local edge case
a5cfe0f 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request May 5, 2024
@ShogunPanda @nodejs-github-bot
src: only apply fix in main thread
de7b378 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@ShogunPanda ShogunPanda deleted the fix-env-test branch May 5, 2024 13:35
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@ShogunPanda
src: fix test local edge case
9d82931 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@ShogunPanda
src: only apply fix in main thread
3062b44 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
targos pushed a commit that referenced this pull request May 8, 2024
@ShogunPanda @targos
src: fix test local edge case
e1cba97 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
targos pushed a commit that referenced this pull request May 8, 2024
@ShogunPanda @targos
src: only apply fix in main thread
9c76c95 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@targos targos mentioned this pull request May 13, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
@ShogunPanda @marco-ippolito
src: fix test local edge case
8fc52b3 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
@ShogunPanda @marco-ippolito
src: only apply fix in main thread
978ee0a 
PR-URL: #52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Jun 17, 2024
Merged
EliphazBouye pushed a commit to EliphazBouye/node that referenced this pull request Jun 20, 2024
@ShogunPanda @EliphazBouye
src: fix test local edge case
cc1e4c1 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this pull request Jun 20, 2024
@ShogunPanda @EliphazBouye
src: only apply fix in main thread
66d2a3d 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@ShogunPanda @bmeck
src: fix test local edge case
cf2df6e 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@ShogunPanda @bmeck
src: only apply fix in main thread
8036f74 
PR-URL: nodejs#52702
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr benjamingr approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@MoLow MoLow MoLow approved these changes

@tniessen tniessen tniessen approved these changes

@jasnell jasnell jasnell approved these changes

@anonrig anonrig Awaiting requested review from anonrig

Assignees
No one assigned
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ShogunPanda @nodejs-github-bot @jasnell @benjamingr @tniessen @MoLow @marco-ippolito